-
Notifications
You must be signed in to change notification settings - Fork 92
gw-advanced-merge-tags.php: Added :selected modifier to target selected multi select choice by index on the Advanced Merge Tags snippet.
#1080
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…lected multi select choice by index on the Advanced Merge Tags snippet.
WalkthroughThe update modifies the internal logic of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GW_Advanced_Merge_Tags
User->>GW_Advanced_Merge_Tags: Call handle_field_modifiers(field, value, modifier)
alt field type is 'checkbox' or 'multiselect' and modifier is 'selected'
GW_Advanced_Merge_Tags->>GW_Advanced_Merge_Tags: Extract index from modifier
GW_Advanced_Merge_Tags->>GW_Advanced_Merge_Tags: Retrieve value at specified index
GW_Advanced_Merge_Tags-->>User: Return selected value
else
GW_Advanced_Merge_Tags-->>User: Process modifier as before
end
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
gravity-forms/gw-advanced-merge-tags.php (1)
496-507: Consider adding a usage example in the comment.While the implementation is solid, it would be helpful to add a brief usage example in the comment to make it clearer for developers how to use this feature with both field types.
// 'selected' can be used over 'Checkbox' or 'Multi Select' field to target the selected checkbox/multiselect choice by its zero-based index. +// Example: {Multi Select Field:1:selected[0]} returns the first selected option if ( $field->type == 'checkbox' || $field->type == 'multiselect' ) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
gravity-forms/gw-advanced-merge-tags.php(1 hunks)
🔇 Additional comments (2)
gravity-forms/gw-advanced-merge-tags.php (2)
496-497: Enhancement looks good - expands:selectedmodifier to work with multi-select fields.The update appropriately extends the
:selectedmodifier functionality to handle both checkbox and multiselect field types. The comment has been properly updated to reflect this expanded functionality, and the condition now correctly checks for both field types.This change aligns perfectly with the PR objective of adding the
:selectedmodifier to target selected multi-select choices by their zero-based index, similar to how it already works with checkboxes.
497-507:❓ Verification inconclusive
Verify the expected output format for multiselect fields.
Ensure that multiselect fields indeed return their values as comma-separated strings like checkboxes do. If they have a different format, additional handling might be needed.
🏁 Script executed:
#!/bin/bash # Search for any related multiselect field processing code rg -A 3 "multiselect.*value" --type phpLength of output: 41
Verify multiselect value formatting
The code below assumes$valuefor multiselect fields is a comma‑separated string (just like checkboxes) and usesexplode(',', $value). Please confirm that Gravity Forms indeed returns multiselect values in that format. If it uses a different delimiter or structure, you’ll need to adjust the parsing logic accordingly.• File: gravity‑forms/gw‑advanced‑merge‑tags.php
• Lines: 497‑507
• Snippet:if ( $field->type == 'checkbox' || $field->type == 'multiselect' ) { $index = $modifier_options; if ( $index !== 'selected' && is_numeric( $index ) ) { $index = intval( $index ); } else { break; } $value_array = explode( ',', $value ); return rgar( $value_array, $index ); }
Context
⛑️ Ticket(s): https://secure.helpscout.net/conversation/2893613491/81558
Summary
Added support for a
:selectedmodifier that would let you target the selected multi select choice by its zero-based index in our Advanced Merge Tags snippet.David's loom:
https://www.loom.com/share/a0f523d97c964e698c7d8b0ee56e2da2
All the documentation we have on the :selected modifier:
https://gravitywiz.com/gravity-wiz-weekly-233/#h-advanced-merge-tags-target-selected-checkboxes
After the update:
https://www.loom.com/share/02471d2ea98d456c8f0a2a41af4da937